testing: do not generate custom build for rng failure test #864
Conversation
L-series
force-pushed
the
randombytes
branch
2 times, most recently
from
b4c5872 to
8edb835
Compare
test/notrandombytes/notrandombytes.c
Outdated
| static uint32_t out[8]; | ||
| static int32_t outleft = 0; | ||
|
|
||
| int randombytes_counter __attribute__((weak)) = 0; |
There was a problem hiding this comment.
Why do we need to change notrandombytes? I was hoping we just don't link that when we build the RNG failure test.
There was a problem hiding this comment.
Sorry, I misunderstood what you meant in the comments on the previous PR. I've reimplemented it so that we do not link against notrandombytes and instead implement a custom randombytes in the rng_fail test. I've also removed the custom config as it is not actually needed. Please let me know if this is closer to what you intended!
Also (off topic), this first implementation made me realise that ld on linux and macos are quite different.. The mac version doesn't have the equivalent of gnu ld's --wrap flag.
L-series
force-pushed
the
randombytes
branch
3 times, most recently
from
8505d60 to
6dadf57
Compare
As a follow up to e15b34f, we modify the rng failure test so that it does not generate a new build. To achieve this we do the following: * Modify compontents and rules to not generate a cusom build. * Remove the custom test_rng_fail config. * Do not link the rng failure test against notrandombytes.c and instead implement our own randombytes. * Updated test function names to use standard crypto_sign_* API instead of mld_* aliases as the mld_* namespace is no longer available as we are now using the default build with MLD_DEFAULT_NAMESPACE_PREFIX. Those functions which are not exposed in this way by default are wrapped with MLD_API_NAMESPACE. Signed-off-by: Andreas Hatziiliou <[email protected]>
| return randombytes(out, outlen); | ||
| /* Fill the output buffer with random bytes */ | ||
| for (i = 0; i < outlen; i++) | ||
| { | ||
| out[i] = (uint8_t)(((unsigned int)rand() * outlen) % 256); | ||
| } |
There was a problem hiding this comment.
I would prefer notrandombytes at the core to remain consistent in terms of the underlying test-PRNG. I see that this leads to a name-clash, though.
One way one would hack this is to #include "notrandombytes.c" and just bracket it with #define randombytes notrandombytes ... #undef randombytes, and then call notrandombytes here.
But perhaps I'm being too pedantic, @mkannwischer WDYT?
hanno-becker
left a comment
hanno-becker
left a comment
There was a problem hiding this comment.
Thank you @L-series, in principle I agree with the approach taken in this PR, which will help to reduce CI runtime.
There are a number of SUPERCOP related changes which don't seem to belong in this PR.
Beyond that, I wonder if there is a way we can use notrandombytes at the core of the instrumented PRNG still.
As a follow up to e15b34f, we modify the rng failure test so that it
does not generate a new build. To achieve this we do the following:
implement our own randombytes.
instead of mld_* aliases as the mld_* namespace is no longer available
as we are now using the default build with
MLD_DEFAULT_NAMESPACE_PREFIX. Those functions which are not exposed
in this way by default are wrapped with MLD_API_NAMESPACE.